Skip to content

Conversation

@thesan
Copy link
Collaborator

@thesan thesan commented Mar 4, 2024

Address part of Joystream/joystream#5088

IMO rate limiting (DDOS prevention...) should be taken cared of by specialized tools (e.g Fail2ban).

affects: @joystream/hydra-cli

Add optional timeout for gql request and db queries

@thesan thesan marked this pull request as draft March 5, 2024 13:03
@thesan thesan requested a review from kdembler March 5, 2024 15:00
@thesan thesan marked this pull request as ready for review March 5, 2024 15:00
@thesan
Copy link
Collaborator Author

thesan commented Mar 5, 2024

@kdembler I had made a mistake on the server config. Now I made a bunch of tests and even though according to the telemetry queries keep on going after the request timed out especially when the timeout is less than 300 ms, it looks like it's working. But the best would be to identify one of the slow queries thank to the telemetry and see what happens when the timeout is set.

@thesan
Copy link
Collaborator Author

thesan commented Mar 5, 2024

Also I set this.httpServer.setTimeout(requestTimeout); in @joystream/warthog directly and it had the exact same effect than using the middleware (i.e the flow is not interrupted immediately but the request does timeout).

@thesan
Copy link
Collaborator Author

thesan commented Mar 8, 2024

Hey @kdembler. You were right: I tested the express middleware with a partially processed mainnet, which made it clear that the flow wasn't stopping at all after the request timed out.

So I tried a similar approach but using a type-graphql global middleware and this time the flow does appear to be aborted. It did take a few 100s of ms after the timeout but with a query that would take ~800ms to execute (without timeout): the last event reported by the telemetry was at ~400ms (for a timeout of 200ms) so it seems to be working now.

@thesan
Copy link
Collaborator Author

thesan commented Mar 8, 2024

Actually it didn't work with the slow distributionBuckets query this one ran in 51s without timing out. So I'll look at it some more on Monday...

That was just due to a the last rename of GRAPHQL_SERVER_RESOLVER_TIMEOUT.

Still I'm seeing some strange results so I'll look some more on Monday.

@thesan thesan marked this pull request as draft March 11, 2024 11:13
…custom maxQueryExecutionTime

affects: @joystream/hydra-cli

Set \`maxQueryExecutionTime\` through an environment variable
@thesan thesan force-pushed the feature/qn-request-timeout branch from 00e6ca4 to d4a124a Compare March 14, 2024 08:57
@thesan thesan changed the title feat(Add optional timeout for gql request and db queries): add timeout Set maxQueryExecutionTime through an environment variable Mar 14, 2024
@thesan thesan marked this pull request as ready for review March 14, 2024 08:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants